Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #2375, Use size_t for variables/parameters representing size #2376

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Fix #2375, Use size_t for variables/parameters representing size #2376

merged 1 commit into from
Jul 2, 2024

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

Testing performed
GitHub CI actions all passing successfully (incl. Build + Run, Unit/Functional Tests etc.).

Expected behavior changes
Some signed variables changed to unsigned but it would have been illogical/impossible to input negative values in those cases.

Contributor Info
Avi Weiss @thnkslprpt

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 15, 2023
View reviewed changes
modules/core_private/ut-stubs/src/ut_support.c
@@ -507,7 +507,7 @@
** But on the upside this concession avoids a far more complicated issue of
** needing a fully-fledged implementation of printf in the OS_printf stub.
*/
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, uint32 FormatLength, uint32 TestLength)
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, size_t FormatLength, size_t TestLength)

Check notice

Code scanning / CodeQL

Function too long

UT_StrCmpFormatStr has too many lines (95, while 60 are allowed).
modules/core_private/ut-stubs/src/ut_support.c
@@ -351,12 +351,12 @@
/*
** Set the CDS size returned by the BSP
*/
uint8 *UT_SetCDSSize(int32 Size)
uint8 *UT_SetCDSSize(size_t Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/core_private/ut-stubs/src/ut_support.c
@@ -334,7 +334,7 @@
/*
** Set the size of the ES reset area
*/
void UT_SetSizeofESResetArea(int32 Size)
void UT_SetSizeofESResetArea(size_t Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/core_private/ut-stubs/src/ut_support.c
@@ -507,7 +507,7 @@
** But on the upside this concession avoids a far more complicated issue of
** needing a fully-fledged implementation of printf in the OS_printf stub.
*/
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, uint32 FormatLength, uint32 TestLength)
static int UT_StrCmpFormatStr(const char *FormatStr, const char *TestStr, size_t FormatLength, size_t TestLength)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/es/ut-coverage/es_UT.c
@@ -460,7 +460,7 @@
}
}

void ES_UT_SetupCDSGlobal(uint32 CDS_Size)
void ES_UT_SetupCDSGlobal(size_t CDS_Size)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/evs/ut-coverage/evs_UT.c
@@ -155,7 +155,7 @@
return StubRetcode;
}

static void UT_EVS_DoDispatchCheckEvents_Impl(void *MsgPtr, uint32 MsgSize, UT_TaskPipeDispatchId_t DispatchId,
static void UT_EVS_DoDispatchCheckEvents_Impl(void *MsgPtr, size_t MsgSize, UT_TaskPipeDispatchId_t DispatchId,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
zbarnett
zbarnett reviewed Feb 16, 2024
View reviewed changes
modules/es/fsw/src/cfe_es_task.c
@@ -1088,8 +1088,8 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data)
OS_close(FileDescriptor);
CFE_ES_Global.TaskData.CommandCounter++;
CFE_EVS_SendEvent(CFE_ES_ALL_APPS_EID, CFE_EVS_EventType_DEBUG,
"App Info file written to %s, Entries=%d, FileSize=%d", QueryAllFilename, (int)EntryCount,
(int)FileSize);
"App Info file written to %s, Entries=%d, FileSize=%lu", QueryAllFilename, (int)EntryCount,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be %zu and then you wouldn't have to cast FileSize at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem is not all "c99" compilers recognize %zu yet.... Once we update to C11 then we should be able to use the z modifier. For now I prefer to keep the cast to unsigned long.

jphickey
jphickey approved these changes May 23, 2024
View reviewed changes
modules/es/fsw/src/cfe_es_task.c
@@ -1088,8 +1088,8 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data)
OS_close(FileDescriptor);
CFE_ES_Global.TaskData.CommandCounter++;
CFE_EVS_SendEvent(CFE_ES_ALL_APPS_EID, CFE_EVS_EventType_DEBUG,
"App Info file written to %s, Entries=%d, FileSize=%d", QueryAllFilename, (int)EntryCount,
(int)FileSize);
"App Info file written to %s, Entries=%d, FileSize=%lu", QueryAllFilename, (int)EntryCount,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem is not all "c99" compilers recognize %zu yet.... Once we update to C11 then we should be able to use the z modifier. For now I prefer to keep the cast to unsigned long.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 23, 2024
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 30, 2024
@dzbaker dzbaker mentioned this pull request Jul 1, 2024
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
4fd8567 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Jul 2, 2024
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
ac6463a 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@dzbaker dzbaker merged commit 6059459 into nasa:main Jul 2, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Jul 2, 2024
@dzbaker
IC: equuleus-rc1+dev13
9468344 
*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
elf2cfetbl equuleus-rc1+dev56

**Includes:**

*cFS*
- #707
- #773

*cFE*
- nasa/cFE#2560
- nasa/cFE#2344
- nasa/cFE#2359
- nasa/cFE#2376
- nasa/cFE#2496
- nasa/cFE#2554
- nasa/cFE#2568
- nasa/cFE#2566

*osal*
- nasa/osal#1456
- nasa/osal#1465

*elf2cfetbl*
- nasa/elf2cfetbl#147
- nasa/elf2cfetbl#124
- nasa/elf2cfetbl#125
- nasa/elf2cfetbl#127

Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Anh Van <avan989@users.noreply.github.com>
@thnkslprpt thnkslprpt deleted the fix-2375-use-size-t-for-size-variables-and-parameters branch July 3, 2024 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zbarnett zbarnett zbarnett left review comments

@jphickey jphickey jphickey approved these changes

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use size_t for variables/parameters representing size
5 participants
@thnkslprpt @zbarnett @jphickey @dzbaker @chillfig